Skip to content

Conversation

@RoboErikG
Copy link
Contributor

@RoboErikG RoboErikG commented Jun 27, 2025

The basics

This fixes the remaining browser tests

The details

Resolves

Fixes #8953

Proposed Changes

Lots of fixes to the browser tests

Reason for Changes

Test Coverage

Documentation

Additional Information

@github-actions github-actions bot added the PR: fix Fixes a bug label Jun 27, 2025
@RoboErikG RoboErikG marked this pull request as ready for review July 7, 2025 18:03
@RoboErikG RoboErikG requested a review from a team as a code owner July 7, 2025 18:03
@RoboErikG RoboErikG requested a review from gonfunko July 7, 2025 18:03
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Jul 7, 2025
});

test('Redo block deletion', async function () {
// TODO(#9029) enable this test once deleting a block doesn't lose focus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fixed in #9195, can this be reenabled now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 328 to 329
for (const input of block.inputList) {
for (const field of input.fieldRow) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a recently-ish introduced getFields() that returns a generator that might be more convenient here, but not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 503 to 505
while (!(await elementInBounds(browser, flyoutBlock))) {
await scrollFlyout(browser, 0, 50);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use WorkspaceSvg.scrollBoundsIntoView(flyoutBlock.getBoundingRectangleWithoutChildren()) rather than scrolling repeatedly by a fixed amount until it's on screen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would seemingly allow you to get rid of elementInBounds, since it should be safe to do unconditionally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took some reworking of a few of the test methods, but does seem to be cleaner and work better overall. =)

return vertInView && horInView;
}, element);
// Unicode escape to close flyout.
await browser.keys(['\uE00C']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be webdriverio.Key.Escape?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here and elsewhere.

RoboErikG added 2 commits July 7, 2025 11:48
- Switch to using scrollBoundsIntoView instead of scrolling the flyout
- Use webdriverio Key.Escape instead of the string code for it
Copy link
Contributor Author

@RoboErikG RoboErikG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated based on comments

});

test('Redo block deletion', async function () {
// TODO(#9029) enable this test once deleting a block doesn't lose focus
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 328 to 329
for (const input of block.inputList) {
for (const field of input.fieldRow) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 503 to 505
while (!(await elementInBounds(browser, flyoutBlock))) {
await scrollFlyout(browser, 0, 50);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took some reworking of a few of the test methods, but does seem to be cleaner and work better overall. =)

return vertInView && horInView;
}, element);
// Unicode escape to close flyout.
await browser.keys(['\uE00C']);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here and elsewhere.

@RoboErikG RoboErikG merged commit 89af298 into RaspberryPiFoundation:develop Jul 8, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser tests fail with "element not interactable"

2 participants